I think you are correct, but we then need to modify the migration testing so it doesn't generate a ton of migration files there, that remain behind and get propagated to the community. Maye it should be using VFS for the test files.

@jim-parry I've updated test in tests/system/Command/SessionsCommandsTest.php in tearDown() function to remove php files under application/Database/Migrations directory.

We'll give it a whirl :)

Won't your change remove al the migrations in application/Database/Migrations?
I thought the purpose of this PR was to prevent that.
Do we not need to use VFS perhaps, for the migration testing, rather than letting them be created in application/,,,? Not sure how easy that is to do.

Yes, the current solution is run on tearDown() function that executed after each test. I'm not sure if using vps will be easy to do, or something in the service probably can be mocked for create migration file.

@jim-parry I've updated the tests to only remove migration file that created via test so it won't remove another existing migration files if any.

Looks like a good solution for now, thanks.
Long-term, we need to add some flexibility so that migrations for testing don't touch the application folder - but not today :-/

I may have spoken too soon. Looking at the migration docs, migrations are created with a timestamp-based name if the "type" preference is "timestamp". That would create migrations starting with "2", eg. 20181119..., which is the set you git ignore. So we can't git ignore them :-/

If SessionsCommandsTest deletes all the ones that it creates, we are good. Testing here.

Oops - misread your change to .gitignore ... you don't keep them around. All is good :) and it tests fine.